Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial cleanup #54

Merged
merged 3 commits into from
Sep 6, 2019
Merged

Editorial cleanup #54

merged 3 commits into from
Sep 6, 2019

Conversation

mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented Sep 5, 2019

This PR is fairly large, but the bulk of the changes are purely editorial fixes. These include:

  • fixes for language consistency between sections;
  • consolidation of prose;
  • spelling and grammar fixes;
  • diversification of examples

The one change of note is to the required manifest properties. I noticed that we still listed title and reading order, but these are not required in the manifest. They are required in the canonical representation, as they always get compiled if not explicit. I've done my best to make the distinction between the manifest requirements and the WebIDL clear, but comments welcome. (See the changes to 2.2 and 2.3.)


Preview | Diff

- differentiate required properties from canonical defaults;
- fixes for language consistency between sections;
- consolidation of prose;
- spelling and grammar fixes;
- diversification of examples
index.html Outdated
<p>The digital publication manifest includes entries to set both these concepts &#8212; both <a
href="#manifest-lang-dir-global">globally</a> and for <a href="#manifest-lang-dir-local"
>individual items</a> &#8212; to aid user agents in interpreting and presenting the
metadata.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"both globally and for individual items": unfortunately, that is not correct for the base direction of individual items. At this point I think the best is to remove that side remark.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but I'd hate to lose the explanation. I'll change to clarify language in the subclause:

The digital publication manifest provides the ability to set both these concepts globally — and, for language, on individual items — to aid user agents in interpreting and presenting the metadata.

as JSON-LD does not currently include facilities for this. As an interim measure, this
specification defines a <code>direction</code> property for this purpose, as described in the
following table.</p>
<p>Unlike the default language, the default text direction cannot be specified in the context as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The global language declaration MUST follow the..." somehow it took me a while to understand what was said that there. Maybe it would help to put in a link to the "publication context" term definition rather than the reference to the section. (Ie, introduce 'publication context' with a <def> tag, or something of that effect)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, the dangling modifier at the end doesn't help, either, as it's the language that isn't required. The publication context always has to be present.

index.html Outdated
manifests to include this.</p>

<p>For more information about localized strings on the Web, refer to [[string-meta]].</p>
</div>

<p>In order to correctly handle manifests entries containing right-to-left or bidirectional text,
user agents SHOULD identify the base direction of any given natural language value by scanning
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether, editorially, it is not better to move this set of paragraphs and notes into a separate appendix, possibly appendix D. It breaks the natural flow of the text at this point, going into much (necessary) details. After all, per the note above, these remarks may become obsolete if the direction is properly handled in RDF/JSON-LD at some points later, making this text a bit unnecessary.

<pre class="example" title="Reading progression set explicitl to ltr">
{
"@context" : ["https://schema.org","https://www.w3.org/ns/pub-context"],
"type" : "Book",
&#8230;
"url" : "https://publisher.example.org/mobydick",
"url" : "https://publisher.example.org/leviathan",
"readingProgression" : "ltr"
}
</pre>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it is worth adding a note whereby later versions of this specifications may add more values to this property, like "down" or "up" for certain type of publications. Maybe @llemeurfr can advise on this (or adding it already now?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's take this up in #51, though. I'm fine adding another value now if it's reasonable that a top-to-bottom progression will be needed, otherwise we need to clarify extensibility as right now the wording wouldn't allow for other values.

index.html Show resolved Hide resolved
well-formed [[!bcp47]] and issue a warning for any invalid values found.</p>
Recursively check that every <var>language</var> property in <var>processed</var> is <a
href="https://tools.ietf.org/html/bcp47#section-2.2.9">valid</a>&#160;[[!bcp47]] and
issue a warning for any invalid values found.</p>
</li>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep to the 'well-formed' for now (that is used in other parts of the spec). The response to the 'valid' question should come from the I18N guys, and, afaik, @r12a will raise an issue about it in an I18N repository. (We may want to add an editorial note here, just that we do not forget about it...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I thought I undid them all! I was intending to leave it alone pending a resolution. I'll run another search on valid and add a note.

invalid values found.</p>
that each value in its array is <a
href="https://tools.ietf.org/html/bcp47#section-2.2.9">valid</a>&#160;[[!bcp47]] and
issue a warning for any invalid values found.</p>
</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments above about 'valid'

@mattgarrish mattgarrish merged commit d13f787 into master Sep 6, 2019
@mattgarrish mattgarrish deleted the editorial/review20190904 branch September 6, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants